fix: refactor: remove legacy MissingNodeDialog (#9698)#9708
Closed
fix: refactor: remove legacy MissingNodeDialog (#9698)#9708
Conversation
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
🎭 Playwright: ❌ 554 passed, 1 failed❌ Failed Tests📊 Browser Reports
|
🎨 Storybook: ✅ Built — View Storybook |
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
💤 Files with no reviewable changes (1)
📝 WalkthroughWalkthroughThis PR removes the legacy Missing Nodes dialog and all its UI, tests, composables, settings, and locale keys. Missing-node handling is consolidated to the executionErrorStore (no dialog is shown by app.ts or workflowService). Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
📦 Bundle: 4.57 MB gzip 🟢 -6.06 kBDetailsSummary
Category Glance App Entry Points — 28.9 kB (baseline 28.9 kB) • 🔴 +54 BMain entry bundles and manifests
Status: 1 added / 1 removed Graph Workspace — 974 kB (baseline 967 kB) • 🔴 +7.15 kBGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 72.4 kB (baseline 72.4 kB) • 🔴 +13 BTop-level views, pages, and routed surfaces
Status: 10 added / 10 removed Panels & Settings — 434 kB (baseline 436 kB) • 🟢 -2.02 kBConfiguration panels, inspectors, and settings screens
Status: 21 added / 21 removed User & Accounts — 16.1 kB (baseline 16.1 kB) • 🔴 +4 BAuthentication, profile, and account management bundles
Status: 6 added / 6 removed Editors & Dialogs — 77.5 kB (baseline 77.5 kB) • 🔴 +4 BModals, dialogs, drawers, and in-app editors
Status: 2 added / 2 removed UI Components — 56.5 kB (baseline 56.5 kB) • 🔴 +5 BReusable component library chunks
Status: 13 added / 13 removed Data & Services — 2.54 MB (baseline 2.77 MB) • 🟢 -225 kBStores, services, APIs, and repositories
Status: 13 added / 14 removed Utilities & Hooks — 254 kB (baseline 56.8 kB) • 🔴 +197 kBHelpers, composables, and utility bundles
Status: 16 added / 15 removed Vendor & Third-Party — 8.88 MB (baseline 8.88 MB) • 🔴 +165 BExternal libraries and shared vendor chunks
Status: 3 added / 3 removed Other — 8.04 MB (baseline 8.04 MB) • 🟢 -52 BBundles that do not match a named category
Status: 74 added / 74 removed |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/platform/workflow/core/services/workflowService.test.ts (1)
203-219: Consider clarifying the test name.The test name says "should not show models dialog when settings are disabled" but the test also verifies that
surfaceMissingNodesIS called regardless of settings. The current name doesn't fully capture both assertions.A more descriptive name could be:
'should surface missing nodes unconditionally while respecting models dialog setting'That said, the test logic correctly validates the behavior from the implementation where
surfaceMissingNodeshas no setting gate whilemissingModelsDialog.showdoes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/platform/workflow/core/services/workflowService.test.ts` around lines 203 - 219, Rename the test case to reflect both assertions: change the it(...) title in the test that calls useWorkflowService().showPendingWarnings(workflow) and asserts useExecutionErrorStore().surfaceMissingNodes(...) and that mockShowMissingModels was not called; for example use "should surface missing nodes unconditionally while respecting models dialog setting" so the test name matches the verified behavior (refer to the test block using showPendingWarnings, surfaceMissingNodes, and mockShowMissingModels).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/platform/workflow/core/services/workflowService.test.ts`:
- Around line 203-219: Rename the test case to reflect both assertions: change
the it(...) title in the test that calls
useWorkflowService().showPendingWarnings(workflow) and asserts
useExecutionErrorStore().surfaceMissingNodes(...) and that mockShowMissingModels
was not called; for example use "should surface missing nodes unconditionally
while respecting models dialog setting" so the test name matches the verified
behavior (refer to the test block using showPendingWarnings,
surfaceMissingNodes, and mockShowMissingModels).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4a752463-7b72-44d4-93f1-b96bfd71884b
📒 Files selected for processing (27)
browser_tests/fixtures/selectors.tsbrowser_tests/tests/dialog.spec.tsbrowser_tests/tests/nodeDisplay.spec.tsbrowser_tests/tests/primitiveNode.spec.tssrc/components/dialog/content/MissingCoreNodesMessage.test.tssrc/components/dialog/content/MissingCoreNodesMessage.vuesrc/components/dialog/content/MissingNodesContent.vuesrc/components/dialog/content/MissingNodesFooter.vuesrc/components/dialog/content/MissingNodesHeader.vuesrc/composables/useMissingNodesDialog.tssrc/locales/ar/settings.jsonsrc/locales/en/settings.jsonsrc/locales/es/settings.jsonsrc/locales/fa/settings.jsonsrc/locales/fr/settings.jsonsrc/locales/ja/settings.jsonsrc/locales/ko/settings.jsonsrc/locales/pt-BR/settings.jsonsrc/locales/ru/settings.jsonsrc/locales/tr/settings.jsonsrc/locales/zh-TW/settings.jsonsrc/locales/zh/settings.jsonsrc/platform/settings/constants/coreSettings.tssrc/platform/workflow/core/services/workflowService.test.tssrc/platform/workflow/core/services/workflowService.tssrc/schemas/apiSchema.tssrc/scripts/app.ts
💤 Files with no reviewable changes (26)
- src/locales/zh/settings.json
- src/locales/ar/settings.json
- src/locales/tr/settings.json
- src/locales/es/settings.json
- browser_tests/tests/primitiveNode.spec.ts
- src/locales/ru/settings.json
- browser_tests/tests/nodeDisplay.spec.ts
- src/locales/fr/settings.json
- browser_tests/fixtures/selectors.ts
- src/schemas/apiSchema.ts
- src/composables/useMissingNodesDialog.ts
- src/locales/pt-BR/settings.json
- src/locales/ko/settings.json
- src/locales/ja/settings.json
- src/components/dialog/content/MissingNodesContent.vue
- src/locales/zh-TW/settings.json
- src/locales/en/settings.json
- src/locales/fa/settings.json
- src/platform/settings/constants/coreSettings.ts
- src/components/dialog/content/MissingCoreNodesMessage.test.ts
- src/platform/workflow/core/services/workflowService.ts
- browser_tests/tests/dialog.spec.ts
- src/components/dialog/content/MissingCoreNodesMessage.vue
- src/components/dialog/content/MissingNodesFooter.vue
- src/components/dialog/content/MissingNodesHeader.vue
- src/scripts/app.ts
⚡ Performance Report
Raw data{
"timestamp": "2026-03-10T06:09:14.653Z",
"gitSha": "07b34018c903f134aa16cc2350ada7ff5e92508e",
"branch": "fix/coderabbit-issue-9698",
"measurements": [
{
"name": "canvas-idle",
"durationMs": 2038.6359999999968,
"styleRecalcs": 125,
"styleRecalcDurationMs": 25.705,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 481.8379999999999,
"heapDeltaBytes": -3236068
},
{
"name": "canvas-idle",
"durationMs": 2024.4710000000055,
"styleRecalcs": 122,
"styleRecalcDurationMs": 20.135,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 398.09299999999996,
"heapDeltaBytes": -3448992
},
{
"name": "canvas-idle",
"durationMs": 2047.476999999958,
"styleRecalcs": 123,
"styleRecalcDurationMs": 21.973000000000003,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 420.99600000000004,
"heapDeltaBytes": -2626320
},
{
"name": "canvas-mouse-sweep",
"durationMs": 2065.2880000000096,
"styleRecalcs": 185,
"styleRecalcDurationMs": 55.80299999999999,
"layouts": 12,
"layoutDurationMs": 3.5469999999999997,
"taskDurationMs": 1056.98,
"heapDeltaBytes": -3622028
},
{
"name": "canvas-mouse-sweep",
"durationMs": 1797.0170000000394,
"styleRecalcs": 163,
"styleRecalcDurationMs": 45.501,
"layouts": 12,
"layoutDurationMs": 3.2140000000000004,
"taskDurationMs": 790.588,
"heapDeltaBytes": -3812620
},
{
"name": "canvas-mouse-sweep",
"durationMs": 1830.9859999999958,
"styleRecalcs": 166,
"styleRecalcDurationMs": 47.169,
"layouts": 12,
"layoutDurationMs": 3.502,
"taskDurationMs": 795.732,
"heapDeltaBytes": -3424084
},
{
"name": "dom-widget-clipping",
"durationMs": 561.3519999999994,
"styleRecalcs": 41,
"styleRecalcDurationMs": 13.681,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 362.07800000000003,
"heapDeltaBytes": 7016072
},
{
"name": "dom-widget-clipping",
"durationMs": 572.3810000000071,
"styleRecalcs": 42,
"styleRecalcDurationMs": 14.285,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 367.55699999999996,
"heapDeltaBytes": 6993776
},
{
"name": "dom-widget-clipping",
"durationMs": 577.6970000000006,
"styleRecalcs": 43,
"styleRecalcDurationMs": 11.422999999999998,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 359.53200000000004,
"heapDeltaBytes": 7925724
},
{
"name": "subgraph-dom-widget-clipping",
"durationMs": 595.2369999999974,
"styleRecalcs": 75,
"styleRecalcDurationMs": 18.985,
"layouts": 1,
"layoutDurationMs": 0.23299999999999993,
"taskDurationMs": 426.3,
"heapDeltaBytes": 15889700
},
{
"name": "subgraph-dom-widget-clipping",
"durationMs": 597.5080000000048,
"styleRecalcs": 73,
"styleRecalcDurationMs": 15.641000000000002,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 411.683,
"heapDeltaBytes": 14115620
},
{
"name": "subgraph-dom-widget-clipping",
"durationMs": 583.3860000000186,
"styleRecalcs": 73,
"styleRecalcDurationMs": 15.056999999999999,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 413.00800000000004,
"heapDeltaBytes": -8641196
},
{
"name": "subgraph-idle",
"durationMs": 2003.8929999999482,
"styleRecalcs": 121,
"styleRecalcDurationMs": 26.125000000000004,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 438.28600000000006,
"heapDeltaBytes": -4294580
},
{
"name": "subgraph-idle",
"durationMs": 2000.6309999999985,
"styleRecalcs": 121,
"styleRecalcDurationMs": 23.725999999999996,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 399.123,
"heapDeltaBytes": -3540336
},
{
"name": "subgraph-idle",
"durationMs": 1989.781999999991,
"styleRecalcs": 120,
"styleRecalcDurationMs": 22.019000000000005,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 390.75399999999996,
"heapDeltaBytes": -3962432
},
{
"name": "subgraph-mouse-sweep",
"durationMs": 1990.5049999999846,
"styleRecalcs": 172,
"styleRecalcDurationMs": 57.945,
"layouts": 16,
"layoutDurationMs": 4.123,
"taskDurationMs": 1037.5240000000001,
"heapDeltaBytes": -5097368
},
{
"name": "subgraph-mouse-sweep",
"durationMs": 1991.3779999999974,
"styleRecalcs": 172,
"styleRecalcDurationMs": 54.768,
"layouts": 16,
"layoutDurationMs": 4.182,
"taskDurationMs": 1002.184,
"heapDeltaBytes": -5065556
},
{
"name": "subgraph-mouse-sweep",
"durationMs": 1707.7239999999847,
"styleRecalcs": 155,
"styleRecalcDurationMs": 43.035,
"layouts": 16,
"layoutDurationMs": 4.11,
"taskDurationMs": 725.3190000000001,
"heapDeltaBytes": -6480356
}
]
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #9698
Summary
The legacy
MissingNodesDialogmodal was redundant — missing node errors are already surfaced throughTabErrorsviaexecutionErrorStore.surfaceMissingNodes(). This PR removes the dialog and all associated code, including theComfy.Workflow.ShowMissingNodesWarningsetting.Changes
src/composables/useMissingNodesDialog.ts— the composable that launched the modalsrc/components/dialog/content/MissingNodesContent.vue,MissingNodesFooter.vue,MissingNodesHeader.vue,MissingCoreNodesMessage.vueand its test — the modal UI componentsComfy.Workflow.ShowMissingNodesWarningsetting fromcoreSettings.tsandapiSchema.tsworkflowService.tsandapp.tsto remove the modal call while keepingsurfaceMissingNodes()calls intactworkflowService.test.tsto test againstexecutionErrorStore.surfaceMissingNodesdirectly instead of the removed dialog mockdialog.spec.ts,primitiveNode.spec.ts, andnodeDisplay.spec.tsthat tested the now-removed modalmissingNodestest ID fromselectors.tsAutomated by coderabbit-fixer
┆Issue is synchronized with this Notion page by Unito